repo: Add new API to write config with reload+validation

The `ostree config set` CLI should really disallow
writing invalid values. To implement that, add a new
API that also *reloads* to the new config, and
rolls back on failure.

Closes: https://github.com/ostreedev/ostree/issues/1827
Signed-off-by: Colin Walters <walters@verbum.org>

diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt
index a43d1079293b03e6d2ddd43898051d67fc6b2ac2..e392f1e09e2b079e3e516955650d723742ba78bd 100644 (file)
--- a/apidoc/ostree-sections.txt
+++ b/apidoc/ostree-sections.txt
@@ -352,6 +352,7 @@ ostree_repo_get_remote_list_option
 ostree_repo_get_remote_option
 ostree_repo_get_parent
 ostree_repo_write_config
+ostree_repo_write_config_and_reload
 OstreeRepoTransactionStats
 ostree_repo_scan_hardlinks
 ostree_repo_prepare_transaction

diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym
index 610a36b7e990e4aa95edf84062c20b8357c6b92d..f7e7eb8b8ea638d4d8b7bb01897eae1b3d3e5917 100644 (file)
--- a/src/libostree/libostree-devel.sym
+++ b/src/libostree/libostree-devel.sym
@@ -32,6 +32,7 @@ global:
 
 LIBOSTREE_2025.3 {
 global:
+  ostree_repo_write_config_and_reload;
   ostree_deployment_is_soft_reboot_target;
   ostree_sysroot_deployment_can_soft_reboot;
   ostree_sysroot_deployment_set_soft_reboot;

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index 4e2f47d0e73aa5f38ffab79eeb112028c7c14c30..9aec76f40b35217b25e6cf5ad2a544eada1c9c85 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -435,6 +435,7 @@ pop_repo_lock (OstreeRepo *self, OstreeRepoLockType lock_type, gboolean blocking
 
   return TRUE;
 }
+static gboolean reload_config_inner (OstreeRepo *self, GCancellable *cancellable, GError **error);
 
 /**
  * ostree_repo_lock_push:
@@ -1566,6 +1567,9 @@ ostree_repo_copy_config (OstreeRepo *self)
  * @error: a #GError
  *
  * Save @new_config in place of this repository's config file.
+ *
+ * Note: This will not validate many elements of the configuration.
+ * Prefer `ostree_repo_write_config_and_reload`.
  */
 gboolean
 ostree_repo_write_config (OstreeRepo *self, GKeyFile *new_config, GError **error)
@@ -1618,6 +1622,36 @@ ostree_repo_write_config (OstreeRepo *self, GKeyFile *new_config, GError **error
   return TRUE;
 }
 
+/**
+ * ostree_repo_write_config_and_reload:
+ * @self: Repo
+ * @new_config: Overwrite the config file with this data, and reload
+ * @error: a #GError
+ *
+ * Save @new_config in place of this repository's config file and reload.
+ * The config will be validated.
+ */
+gboolean
+ostree_repo_write_config_and_reload (OstreeRepo *self, GKeyFile *new_config, GError **error)
+{
+  g_return_val_if_fail (self->inited, FALSE);
+
+  g_autoptr (GKeyFile) old_config = g_steal_pointer (&self->config);
+  // Test reloading with the new config
+  self->config = new_config;
+  gboolean r = reload_config_inner (self, NULL, error);
+  self->config = g_steal_pointer (&old_config);
+  if (!r)
+    {
+      // Best effort to revert back to the old config, but if that fails
+      // we're in a doubly bad state.
+      (void)reload_config_inner (self, NULL, NULL);
+      return FALSE;
+    }
+  // Now perform the actual write
+  return ostree_repo_write_config (self, new_config, error);
+}
+
 /* Bind a subset of an a{sv} to options in a given GKeyfile section */
 static void
 keyfile_set_from_vardict (GKeyFile *keyfile, const char *section, GVariant *vardict)
@@ -2993,19 +3027,6 @@ reload_core_config (OstreeRepo *self, GCancellable *cancellable, GError **error)
   g_autofree char *contents = NULL;
   g_autofree char *parent_repo_path = NULL;
   gboolean is_archive;
-  gsize len;
-
-  g_clear_pointer (&self->config, g_key_file_unref);
-  self->config = g_key_file_new ();
-
-  contents = glnx_file_get_contents_utf8_at (self->repo_dir_fd, "config", &len, NULL, error);
-  if (!contents)
-    return FALSE;
-  if (!g_key_file_load_from_data (self->config, contents, len, 0, error))
-    {
-      g_prefix_error (error, "Couldn't parse config file: ");
-      return FALSE;
-    }
 
   version = g_key_file_get_value (self->config, "core", "repo_version", error);
   if (!version)
@@ -3353,6 +3374,18 @@ reload_sysroot_config (OstreeRepo *self, GCancellable *cancellable, GError **err
   return TRUE;
 }
 
+static gboolean
+reload_config_inner (OstreeRepo *self, GCancellable *cancellable, GError **error)
+{
+  if (!reload_core_config (self, cancellable, error))
+    return FALSE;
+  if (!reload_remote_config (self, cancellable, error))
+    return FALSE;
+  if (!reload_sysroot_config (self, cancellable, error))
+    return FALSE;
+  return TRUE;
+}
+
 /**
  * ostree_repo_reload_config:
  * @self: repo
@@ -3367,13 +3400,21 @@ reload_sysroot_config (OstreeRepo *self, GCancellable *cancellable, GError **err
 gboolean
 ostree_repo_reload_config (OstreeRepo *self, GCancellable *cancellable, GError **error)
 {
-  if (!reload_core_config (self, cancellable, error))
-    return FALSE;
-  if (!reload_remote_config (self, cancellable, error))
-    return FALSE;
-  if (!reload_sysroot_config (self, cancellable, error))
+  g_clear_pointer (&self->config, g_key_file_unref);
+  self->config = g_key_file_new ();
+
+  gsize len;
+  g_autofree char *contents
+      = glnx_file_get_contents_utf8_at (self->repo_dir_fd, "config", &len, NULL, error);
+  if (!contents)
     return FALSE;
-  return TRUE;
+  if (!g_key_file_load_from_data (self->config, contents, len, 0, error))
+    {
+      g_prefix_error (error, "Couldn't parse config file: ");
+      return FALSE;
+    }
+
+  return reload_config_inner (self, cancellable, error);
 }
 
 gboolean

diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h
index d38fad9a2b7d34fc9188ffd64a425769df1cede6..460188b008adff692ea5e467fa98b63091248d1a 100644 (file)
--- a/src/libostree/ostree-repo.h
+++ b/src/libostree/ostree-repo.h
@@ -192,6 +192,9 @@ OstreeRepo *ostree_repo_get_parent (OstreeRepo *self);
 
 _OSTREE_PUBLIC
 gboolean ostree_repo_write_config (OstreeRepo *self, GKeyFile *new_config, GError **error);
+_OSTREE_PUBLIC
+gboolean ostree_repo_write_config_and_reload (OstreeRepo *self, GKeyFile *new_config,
+                                              GError **error);
 
 /**
  * OstreeRepoCommitState:

diff --git a/src/ostree/ot-builtin-config.c b/src/ostree/ot-builtin-config.c
index 039656c5898d8a3e8dfafb357a5772d5a3d244c1..c269a40ef8bb5c77b2486bc5338fc43791efa5c8 100644 (file)
--- a/src/ostree/ot-builtin-config.c
+++ b/src/ostree/ot-builtin-config.c
@@ -116,7 +116,7 @@ ostree_builtin_config (int argc, char **argv, OstreeCommandInvocation *invocatio
       config = ostree_repo_copy_config (repo);
       g_key_file_set_string (config, section, key, value);
 
-      if (!ostree_repo_write_config (repo, config, error))
+      if (!ostree_repo_write_config_and_reload (repo, config, error))
         return FALSE;
     }
   else if (!strcmp (op, "get"))

diff --git a/tests/test-config.sh b/tests/test-config.sh
index 2d9aaf5337f59f0a9f7f42f94b2c7f1f9d3970f7..d4a35b4caa3a31b47be6def95db529d88712be28 100755 (executable)
--- a/tests/test-config.sh
+++ b/tests/test-config.sh
@@ -21,7 +21,7 @@ set -euo pipefail
 
 . $(dirname $0)/libtest.sh
 
-echo '1..3'
+echo '1..4'
 
 ostree_repo_init repo
 ${CMD_PREFIX} ostree remote add --repo=repo --set=xa.title=Flathub --set=xa.title-is-set=true flathub https://dl.flathub.org/repo/
@@ -97,3 +97,20 @@ if ${CMD_PREFIX} ostree config --repo=repo unset core.lock-timeout-secs extra 2>
 fi
 assert_file_has_content err.txt "error: Too many arguments given"
 echo "ok config unset"
+
+# Test config validation - should reject invalid values immediately
+if ${CMD_PREFIX} ostree config --repo=repo set core.min-free-space-size "invalidvalue" 2>err.txt; then
+    assert_not_reached "ostree config set should reject invalid core.min-free-space-size"
+fi
+assert_file_has_content err.txt "Invalid min-free-space-size"
+
+# Set a valid value, now try resetting it
+ostree config --repo=repo set core.min-free-space-size "100MB"
+if ${CMD_PREFIX} ostree config --repo=repo set core.min-free-space-size "invalidvalue" 2>err.txt; then
+    assert_not_reached "ostree config set should reject invalid core.min-free-space-size"
+fi
+assert_file_has_content err.txt "Invalid min-free-space-size"
+v=$(${CMD_PREFIX} ostree config --repo=repo get core.min-free-space-size)
+assert_streq "$v" "100MB"
+
+echo "ok config validation"